fix: Types after preact 10.25 #1803
Conversation
|
Thanks for taking a look @rschristian! |
rschristian
changed the title
|
@acelaya Preact 10.25.1 is out will all the fixes, should be good to go-ish. You still have an issue with passing an Feel free to make additional edits, copy/paste from here, whatever. And of course ping if you run into any more trouble with the types. We've made a big change in recent versions but hope to get everyone switched over without too much difficulty. |
| @@ -41,7 +41,7 @@ type ComponentProps = { | |||
| }; | |||
|
|
|||
| export type HTMLButtonAttributes = Omit< | |||
| JSX.HTMLAttributes<HTMLButtonElement>, | |||
| JSX.ButtonHTMLAttributes<HTMLButtonElement>, | |||
There was a problem hiding this comment.
Just have a question. Are the new JSX.ButtonHTMLAttributes, JSX.InputHTMLAttributes, etc., available in preact versions older than 10.25?
If not, we may need to bump the minimum required preact version of this library, since we ship type definitions with it.
There was a problem hiding this comment.
Great question: they've were available from preact/compat since 10.23 but are only available from preact since 10.25.
I see only now there was a peer dependency on preact, sorry! In that case, indeed, you'll probably need a bump.
You might be able to work out a system if you need to keep compat with older versions, though it might require some tooling. Essentially, the HTMLAttributes used here was a mega interface of all possible attributes (across all elements) which has been renamed to AllHTMLAttributes in v10.25+. HTMLAttributes in v10.25+ therefore is the purely global attributes/props, things like class, id, etc. You could probably set up some sort of fallback situation there if you needed.
There was a problem hiding this comment.
Thanks! Bumping the minimum required version should be fine 🙂
|
Thanks @rschristian! This is much appreciated. We just realized our workflows are not configured to run on pull requests, but on push only (we usually create PRs here directly), so continuous integration didn't trigger. I'll clone this locally, fix the |
acelaya
force-pushed
the
fix/types-after-preact-10.25
branch
from
c300b75 to
d6e9226
Compare
| @@ -60,7 +60,7 @@ | |||
| "yalc": "^1.0.0-pre.50" | |||
| }, | |||
| "peerDependencies": { | |||
| "preact": "^10.4.0" | |||
| "preact": "^10.25.1" | |||
There was a problem hiding this comment.
Setting minimum required version to 10.25.1 rather than 10.25.0, since it includes some important type fixes.
| <LinkButton href="https://www.example.com" underline="hover"> | ||
| Underline: hover | ||
| </LinkButton> | ||
| <LinkButton underline="hover">Underline: hover</LinkButton> | ||
|
|
||
| <LinkButton href="https://www.example.com" underline="always"> | ||
| Underline: always | ||
| </LinkButton> | ||
| <LinkButton underline="always">Underline: always</LinkButton> |
There was a problem hiding this comment.
The href attribute was not used in LinkButton, so it's safe to remove.
These were probably some old examples from a time in which it may have supported it.
acelaya
force-pushed
the
fix/types-after-preact-10.25
branch
from
d6e9226 to
804fc1d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1803 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 68 68
Lines 1234 1234
Branches 468 468
=========================================
Hits 1234 1234 ☔ View full report in Codecov by Sentry. |
My attempt to help out with #1797
The new per-element type interfaces introduces in Preact 10.25 should provide better type assistance but does require a switch over. Additionally, whilst I'm not familiar with this code base I stumbled upon what looks like docs while grepping through; I took the opportunity to update those (seemingly) accordingly?
There have been a few attribute fixes upstream already since 10.25, but not yet released. This PR is therefore a result of manually copying over the type changes since to see if there are any issues we hadn't yet caught. As such, this is based on an in-progress 10.25.1, not really 10.25.0.
There's a few issues remaining:
src/components/icons/SpinnerCircle.tsx: Propertytypedoes not exist...Upstream issue, was missing on the interface. Will get that correctedFIXED, will go out in next versionsrc/pattern-library/components/patterns/navigation/LinkButtonPage.tsx: Propertyhrefdoes not exist...hrefto a buttonsrc/components/input/Checkbox.tsx: Propertycalldoes not exist...This one's a bit weirder and I need some time with yet. I'm not quite sure what's going wrong, but I'd assume it's an upstream issue with our types.FIXED, will also go out in the next version